Runtime rules engine ⚙️ #249
Conversation
Still need caseinsensitive on rule
tdumitrescu
left a comment
tdumitrescu
left a comment
There was a problem hiding this comment.
Thanks for the thorough test cases. Mostly just some nitpicks on my end!
| } else { | ||
| return Object.fromEntries( | ||
| Object.entries(obj).map(([k, v]) => [ | ||
| k.toLowerCase(), |
There was a problem hiding this comment.
This appears to be the only difference between this util and lowercaseLeafNodes - can we consolidate into a single implementation? maybe something like lowercaseObjectEntries({lowercaseKeys: true})
| "dependencies": { | ||
| "https-proxy-agent": "7.0.6" | ||
| "https-proxy-agent": "7.0.6", | ||
| "json-logic-js": "^2.0.5" |
There was a problem hiding this comment.
pin non-dev deps
| "json-logic-js": "^2.0.5" | |
| "json-logic-js": "2.0.5" |
| const otherFlag = createTestFlag({ flagKey: "other_flag" }); | ||
| mockFlagDefinitionsResponse([otherFlag]); | ||
| await provider.startPollingForDefinitions(); | ||
| await createFlagAndLoadItIntoSDK({ flagKey: "other_flag" }, provider); |
| await createFlagAndLoadItIntoSDK({ runtimeEvaluationRule }, provider); | ||
|
|
||
| const context = userContextWithRuntimeParameters({ | ||
| plan: randomString(), |
There was a problem hiding this comment.
| plan: randomString(), | |
| plan: randomString() + "x", |
(will just ensure that regardless of the implementation of randomString() you'll never get plan: "Premium")
| }); | ||
|
|
||
| it("should return fallback when runtime rule is invalid", async () => { | ||
| const runtimeEvaluationRule = { "=oops=": [{ var: "plan" }, "Premium"] }; |
There was a problem hiding this comment.
Might be worth adding debug logging for this case? (at least when the rule is loaded over the network, probably too noisy to do it on every check)
| }); | ||
|
|
||
| const result = provider.getVariant(FLAG_KEY, FALLBACK, context); | ||
| expect(result.variant_value).not.toBe(FALLBACK_NAME); |
There was a problem hiding this comment.
Can we look for a specific value here rather than just "not the fallback"? Since that could pass the test for weird failure cases like if variant_value is unexpectedly null
| it("should return fallback when runtime evaluation not satisfied", async () => { | ||
| const runtimeEval = { plan: "premium", region: "US" }; | ||
| const flag = createTestFlag({ runtimeEvaluation: runtimeEval }); | ||
| it("should return variant when runtime evaluation rule case-insensitively satisfied", async () => { |
There was a problem hiding this comment.
Logically this test case isn't any different than the one on L318 is it?
| await createFlagAndLoadItIntoSDK({ runtimeEvaluationRule }, provider); | ||
|
|
||
| const context = userContextWithRuntimeParameters({ | ||
| name: "d", |
There was a problem hiding this comment.
might be worth adding a test case where this value is a substring of one of the array values e.g. name: "all"
| expect(result.variant_value).toBe(FALLBACK_NAME); | ||
| }); | ||
|
|
||
| it("should return variant when runtime evaluation with and operator satisfied", async () => { |
There was a problem hiding this comment.
a little easier to parse?
| it("should return variant when runtime evaluation with and operator satisfied", async () => { | |
| it("should return variant when runtime evaluation with AND operator satisfied", async () => { |
| mockFlagDefinitionsResponse([flag]); | ||
| await provider.startPollingForDefinitions(); | ||
| const context = userContextWithRuntimeParameters({ | ||
| plan: "premium", |
There was a problem hiding this comment.
Apologies if I missed it, but let's make sure to have a test case that specifies the behavior when the case of the key is mismatched e.g. {PLAN: "premium"}
Background
Local evaluation already supports evaluating the legacy runtime rule, which was simple exact key-value matching.
This pr
Support complex runtime rules in local-evaluation using jsonLogic.
QA'd local and remote eval ✅
https://github.com/mixpanel/sdk-integration-testing/commit/fe6d0968da0d4f30ce482abf46906d82bf7cc373
Example of what they look like in the UI